Skip to content

feat(go/adbc): refactor logging instrumentation into OTel tracing - part 1/3 - #4655

Open
birschick-bq wants to merge 7 commits into
apache:mainfrom
birschick-bq:dev/birschick-bq/flight-sql-log-to-trace-3-reader
Open

feat(go/adbc): refactor logging instrumentation into OTel tracing - part 1/3#4655
birschick-bq wants to merge 7 commits into
apache:mainfrom
birschick-bq:dev/birschick-bq/flight-sql-log-to-trace-3-reader

Conversation

@birschick-bq

@birschick-bq birschick-bq commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

This pull request introduces OpenTelemetry tracing to the FlightSQL driver, enhancing observability for record reading and endpoint streaming operations. The main changes involve adding tracing hooks, attributes, and error recording to critical code paths, as well as refactoring logging and context management to support tracing. Additionally, new utility functions for collecting and attaching response metadata to traces are implemented.

Tracing and Observability Enhancements:

  • Added OpenTelemetry tracing support to record_reader.go, including span creation, event recording, and error tracking in newRecordReader and endpoint goroutines. This enables detailed tracing of FlightSQL record reading operations. [1] [2] [3] [4]
  • Introduced new tracing utility functions and types in flightsql_tracing.go for collecting response metadata, building trace attributes for endpoints, and summarizing stream progress as OpenTelemetry attributes.

Refactoring for Tracing Integration:

  • Replaced the logger-based endpoint and stream progress attribute builders in logging.go with tracing attribute builders, and removed now-redundant logging functions. [1] [2]
  • Updated context and cancellation handling in record_reader.go to use context.CancelCauseFunc for improved error propagation and tracing. [1] [2]

Internal API and Dependency Updates:

  • Added OpenTelemetry and internal tracing imports to relevant files to support the new tracing features. [1] [2] [3]
  • Extended the recordReaderConfig struct to include a tracing configuration parameter, enabling tracing to be passed through to record readers.

These changes collectively provide fine-grained tracing and error visibility for FlightSQL operations, making it easier to monitor, debug, and analyze the driver's behavior in production environments.
Refactors slog instrumentation into OTel tracing.

  • update utilities to instrument duration for a span from a given start time.
  • update utilities to separate already recorded error to avoid duplicate events in the span
  • adds a flightsql_tracing.go to provide tracing wrappers originally provided in logging.go
  • some updates in driverbase to handle improved trace handling
  • some initial instrumentation in flightsql connection and reader

Part 1 of a multi-part change to refactor logging instrumentation into OTel tracing.

Stacked PRs in my personal forked repo ...
Part 2
Part 3

@birschick-bq
birschick-bq requested a review from zeroshade as a code owner August 4, 2026 20:27
@birschick-bq birschick-bq changed the title feat(go/adb): refactor logging instrumentation into OTel tracing - part 1/3 feat(go/adbc): refactor logging instrumentation into OTel tracing - part 1/3 Aug 4, 2026
@lidavidm

lidavidm commented Aug 5, 2026

Copy link
Copy Markdown
Member

Is there some more meaningful way to split this up than part 1/2/3? Can we break it into parts that can each have meaningful descriptions? It's not clear to me how these changes were grouped

@birschick-bq

Copy link
Copy Markdown
Contributor Author

Is there some more meaningful way to split this up than part 1/2/3? Can we break it into parts that can each have meaningful descriptions? It's not clear to me how these changes were grouped

Yes. I'll work on it.

@lidavidm

lidavidm commented Aug 5, 2026

Copy link
Copy Markdown
Member

Ok, thanks. Let me know if that's too much of a hassle. I wish the stacked PRs feature worked for us...I gave it a try, but it means you/others need push access to this repo which is a non-starter (even for committers I'd rather we not push dev branches to this repo). Hopefully that gets fixed.

@birschick-bq

birschick-bq commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Ok, thanks. Let me know if that's too much of a hassle. I wish the stacked PRs feature worked for us...I gave it a try, but it means you/others need push access to this repo which is a non-starter (even for committers I'd rather we not push dev branches to this repo). Hopefully that gets fixed.

Without a re-write here are the individual stacked PRs for part 2 and 3 on my repo
Part 2
Part 3

@lidavidm - Let me know if this is satisfactory. Thanks.

@birschick-bq

Copy link
Copy Markdown
Contributor Author

@davidhcoe - if you have time to review, this refactors your logging instrumentation into OTel tracing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants